Skip to content

Licensed channel defaults, phone map growth, and payload read bounds - #11286

Merged
thebentern merged 3 commits into
developfrom
security-block-e
Jul 29, 2026
Merged

Licensed channel defaults, phone map growth, and payload read bounds#11286
thebentern merged 3 commits into
developfrom
security-block-e

Conversation

@caveman99

@caveman99 caveman99 commented Jul 29, 2026

Copy link
Copy Markdown
Member

Last three follow-ups from the post-release backlog, one commit each. Independent of #11282, #11283, #11284 and #11285; all five branch from develop and merge cleanly in any order.

  • resetRadioConfig() installs the default channels, which carry the public PSK, but only loadFromDisk() sanitized them. When the channel file is absent or corrupt that call is a no-op (channels_count is 0), so a first boot or factory reset of a ham-only board could transmit encrypted on ham allocations until the next boot. Now stripped again right after initDefaults(), before onConfigChanged() publishes hashes.
  • lastPortNumToRadio was written for every portnum a client sent, but only traceroute, position, waypoint, alert, telemetry and text are ever read back. A client cycling portnums grew the map without bound; it now records only those six.
  • payload.bytes is not NUL-terminated, and two readers scanned past the received size into whatever the previous occupant of the pooled packet left behind: MessageStore::addFromPacket bounded strnlen only by MAX_MESSAGE_SIZE, and DropzoneModule compared strlen(matchCompare) bytes regardless of payload.size. Both are now bounded by the received size. In-field reads, so no OOB, but they leaked stale bytes into stored text and into a match decision. The two sprintf calls there became snprintf while in the area.

Item 18 from the backlog (splitting manually-verified from XEdDSA signer provenance in the warm store) needs no change: d6b12ea already added WarmProtected::XeddsaSigner alongside the independent signer bit, so the two are distinguished.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented message processing from reading beyond the received payload.
    • Improved handling of configuration resets on licensed devices, preserving the correct operating settings.
    • Prevented excessive resource usage caused by unsupported packet types.
    • Strengthened received-message matching to safely handle payloads that are not null-terminated.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ccd0967e-0a56-4fd5-b2ba-cb8299412f88

📥 Commits

Reviewing files that changed from the base of the PR and between 50a37b3 and 3e77508.

📒 Files selected for processing (4)
  • src/MessageStore.cpp
  • src/mesh/NodeDB.cpp
  • src/mesh/PhoneAPI.cpp
  • src/modules/DropzoneModule.cpp

📝 Walkthrough

Walkthrough

The changes bound packet parsing to received payload sizes, preserve licensed channel configuration during default initialization, and restrict port timestamp tracking to rate-limited port numbers.

Changes

Safety hardening

Layer / File(s) Summary
Bounded payload handling
src/MessageStore.cpp, src/modules/DropzoneModule.cpp
Message text scanning and Dropzone condition matching now respect received payload lengths and use bounded string operations.
Licensed channel initialization
src/mesh/NodeDB.cpp
Default channel initialization applies licensed-operation settings when the owner is licensed.
Rate-limit port tracking
src/mesh/PhoneAPI.cpp
Timestamp tracking is limited to explicitly rate-limited port numbers.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: bugfix, needs-review

Suggested reviewers: thebentern, jp-bennett

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is substantive but does not follow the required template and omits the attestations/checklist section. Replace the template text with the PR summary, add the attestations checklist, and note testing or any lack of hardware testing.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the PR’s main changes without extra noise.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security-block-e

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

⚡ Try this PR in the Web Flasher

Flash this PR in the Web Flasher

firmware commit boards expires

Warning

This is an automated, unreviewed CI test build. Back up your device configuration
before flashing, and only flash devices you are able to recover.

Supported boards built by this PR (31)
Device Board Platform
Crowpanel Adv 3.5 TFT elecrow-adv-35-tft esp32-s3
Heltec HT62 heltec-ht62-esp32c3-sx1262 esp32-c3
Heltec Mesh Node 096 heltec-mesh-node-t096 nrf52840
Heltec Mesh Node T1 heltec-mesh-node-t1 nrf52840
Heltec Mesh Node T114 heltec-mesh-node-t114 nrf52840
Heltec V3 heltec-v3 esp32-s3
Heltec V4 heltec-v4 esp32-s3
Meshnology W10 meshnology_w10 esp32-s3
Meshnology W12 meshnology_w12 esp32-s3
Raspberry Pi Pico pico rp2040
Raspberry Pi Pico W picow rp2040
RAK WisMesh Pocket V3 rak_wismesh_pocket nrf52840
RAK WisMesh Pod rak_wismesh_pod nrf52840
RAK WisMesh Repeater Mini V2 rak_wismesh_repeater_mini nrf52840
RAK WisMesh Tag rak_wismeshtag nrf52840
RAK WisBlock 11200 rak11200 esp32
RAK WisBlock 11310 rak11310 rp2040
RAK3312 rak3312 esp32-s3
RAK WisBlock 4631 rak4631 nrf52840
Seeed SenseCAP Mesh-Tracker-X1 seeed_mesh_tracker_X1 nrf52840
Seeed Wio Tracker L1 seeed_wio_tracker_L1 nrf52840
Seeed Xiao NRF52840 Kit seeed_xiao_nrf52840_kit nrf52840
Seeed Xiao ESP32-S3 seeed-xiao-s3 esp32-s3
Station G2 station-g2 esp32-s3
Station G3 station-g3 esp32-s3
LILYGO T-Deck t-deck-tft esp32-s3
LILYGO T-Echo t-echo nrf52840
LILYGO T-Echo Plus t-echo-plus nrf52840
LILYGO T-Impulse Plus t-impulse-plus nrf52840
LilyGo T3-C6 tlora-c6 esp32-c6
Seeed SenseCAP T1000-E tracker-t1000-e nrf52840

Build artifacts expire on 2026-08-28. Updated for 3e77508.

@thebentern thebentern added the bugfix Pull request that fixes bugs label Jul 29, 2026
@thebentern
thebentern added this pull request to the merge queue Jul 29, 2026
Merged via the queue into develop with commit daf1213 Jul 29, 2026
102 of 104 checks passed
@caveman99
caveman99 deleted the security-block-e branch July 30, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants